Skip to content

Conversation

@gengliangwang
Copy link
Member

What changes were proposed in this pull request?

Currently, when numbers are divided by 0 under ANSI mode, the error message is like

[DIVIDE_BY_ZERO] Division by zero. Use `try_divide` to tolerate divisor being 0 and return NULL instead. If necessary set "ansi_mode" to "false" (except for ANSI interval type) to bypass this error.

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: "INTERVAL_DIVIDED_BY_ZERO"

Why are the changes needed?

For better error messages

Does this PR introduce any user-facing change?

Yes, Use different error classes for numeric/interval divided by 0. After changes, the error messages are simpler and more clear.

How was this patch tested?

UT

@gengliangwang
Copy link
Member Author

cc @entong as well

},
"INTERVAL_DIVIDED_BY_ZERO" : {
"message" : [
"Division by zero."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't try_divide help here? The TryDivide expression has some examples with intervals:

> SELECT _FUNC_(interval 2 month, 2);
0-1
> SELECT _FUNC_(interval 2 month, 0);
NULL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. I have updated the message. Thanks!

@MaxGekk
Copy link
Member

MaxGekk commented Jul 28, 2022

+1, LGTM. Merging to master (try to 3.3).
Thank you, @gengliangwang and @cloud-fan for review.

@MaxGekk MaxGekk closed this in 5420562 Jul 28, 2022
@MaxGekk
Copy link
Member

MaxGekk commented Jul 28, 2022

@gengliangwang It conflicts w/ 3.3. It is up to you to backport it to branch-3.3.

gengliangwang added a commit that referenced this pull request Aug 1, 2022
…rithmetic overflow

### What changes were proposed in this pull request?

Similar with #37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
```
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"
```

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: `INTERVAL_ARITHMETIC_OVERFLOW`

### Why are the changes needed?

For better error messages

### Does this PR introduce _any_ user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.

### How was this patch tested?

UT

Closes #37337 from gengliangwang/improveOverflowMsg.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
@gengliangwang
Copy link
Member Author

@MaxGekk Thanks, I will keep it on master branch for now.

gengliangwang added a commit to gengliangwang/spark that referenced this pull request Aug 2, 2022
…rithmetic overflow

### What changes were proposed in this pull request?

Similar with apache#37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
```
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"
```

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: `INTERVAL_ARITHMETIC_OVERFLOW`

### Why are the changes needed?

For better error messages

### Does this PR introduce _any_ user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.

### How was this patch tested?

UT

Closes apache#37337 from gengliangwang/improveOverflowMsg.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
gengliangwang added a commit that referenced this pull request Aug 2, 2022
…rithmetic overflow

### What changes were proposed in this pull request?

Similar with #37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
```
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"
```

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: `INTERVAL_ARITHMETIC_OVERFLOW`

### Why are the changes needed?

For better error messages

### Does this PR introduce _any_ user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.

### How was this patch tested?

UT

Closes #37374 from gengliangwang/SPARK-39917.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants